Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Multisite Setup admin wizard for single‑site installs, adjusts setup redirection logic for multisite vs single‑site flows, wires the new wizard into initialization, adds multisite guards and rewrite‑rule flushes in installers/helpers, fixes a docblock typo, and applies minor formatting and truthiness tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant WP as "WP Admin"
participant SW as "Setup_Wizard_Admin_Page"
participant MS as "Multisite_Setup_Admin_Page"
Admin->>WP: Access admin
WP->>SW: redirect_to_wizard()
alt Multisite disabled
SW-->>WP: wp_redirect(admin.php?page=wp-ultimo-multisite-setup)
WP->>MS: Load Multisite wizard (welcome)
else Multisite enabled AND setup not finished
SW-->>WP: wp_redirect(wu_network_admin_url('wp-ultimo-setup'))
else Multisite enabled AND setup finished
SW-->>WP: No redirect
end
sequenceDiagram
autonumber
actor Admin
participant MS as "Multisite_Setup_Admin_Page"
participant FS as "Filesystem"
participant WPNet as "WordPress Network API"
Admin->>MS: Submit configuration (structure, sitename, email)
MS->>MS: Validate input & store transient state
MS->>FS: modify_wp_config() — write initial multisite constants
alt Write successful
MS->>WPNet: create_network(subdomain?, sitename, email)
alt Network created
MS->>FS: add_final_multisite_constants()
MS-->>Admin: Redirect to Complete
else Creation failed
MS-->>Admin: Show manual instructions
end
else Write failed
MS-->>Admin: Show manual instructions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
inc/class-wp-ultimo.php (1)
143-147: Gate Multisite Setup page instantiation to non‑multisite to avoid duplicate top‑level menusRight now both Setup Wizard and Multisite Setup pages register as top-level in single-site, causing duplicate “Multisite Ultimate” menus. Instantiate the Multisite Setup page only when multisite is disabled.
- /* - * Multisite Setup for non-multisite installations - */ - new WP_Ultimo\Admin_Pages\Multisite_Setup_Admin_Page(); + /* + * Multisite Setup for non-multisite installations + */ + if (! is_multisite()) { + new WP_Ultimo\Admin_Pages\Multisite_Setup_Admin_Page(); + }inc/admin-pages/class-multisite-setup-admin-page.php (2)
382-394: Subdomain vs subdirectory .htaccess rules differProvide rules based on selected structure; current rules are subdirectory-only.
- $htaccess_rules = 'RewriteEngine On + $htaccess_rules = $subdomain_install ? 'RewriteEngine On +RewriteBase / +RewriteRule ^index\.php$ - [L] + +RewriteRule ^wp-admin$ wp-admin/ [R=301,L] +RewriteCond %{REQUEST_FILENAME} -f [OR] +RewriteCond %{REQUEST_FILENAME} -d +RewriteRule ^ - [L] +RewriteRule ^(wp-(content|admin|includes).*) $1 [L] +RewriteRule ^(.*\.php)$ $1 [L] +RewriteRule . index.php [L]' : 'RewriteEngine On RewriteRule ^index\.php$ - [L] # add a trailing slash to /wp-admin RewriteRule ^([_0-9a-zA-Z-]+/)?wp-admin$ $1wp-admin/ [R=301,L] RewriteCond %{REQUEST_FILENAME} -f [OR] RewriteCond %{REQUEST_FILENAME} -d RewriteRule ^ - [L] RewriteRule ^([_0-9a-zA-Z-]+/)?(wp-(content|admin|includes).*) $2 [L] RewriteRule ^([_0-9a-zA-Z-]+/)?(.*\.php)$ $2 [L] RewriteRule . index.php [L]';Please verify these match WordPress Network Setup output for your server environment.
Also applies to: 368-375
74-83: Optional: Auto-redirect away if multisite is already enabledPrevents showing this page after success.
public function __construct() { + if (is_multisite()) { + wp_safe_redirect(wu_network_admin_url('wp-ultimo-setup')); + exit; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
inc/admin-pages/class-base-admin-page.php(1 hunks)inc/admin-pages/class-multisite-setup-admin-page.php(1 hunks)inc/admin-pages/class-setup-wizard-admin-page.php(1 hunks)inc/class-wp-ultimo.php(1 hunks)inc/installers/class-default-content-installer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
inc/class-wp-ultimo.php (1)
inc/admin-pages/class-multisite-setup-admin-page.php (1)
Multisite_Setup_Admin_Page(21-626)
inc/admin-pages/class-setup-wizard-admin-page.php (2)
inc/class-requirements.php (1)
run_setup(101-120)inc/functions/helper.php (1)
wu_request(132-137)
inc/admin-pages/class-multisite-setup-admin-page.php (4)
inc/admin-pages/class-wizard-admin-page.php (3)
Wizard_Admin_Page(23-392)render_submit_box(373-383)get_next_section_link(266-275)inc/admin-pages/class-base-admin-page.php (4)
__construct(173-205)get_title(718-718)get_menu_title(726-726)register_scripts(694-694)inc/functions/helper.php (1)
wu_request(132-137)inc/functions/url.php (1)
wu_network_admin_url(53-60)
inc/installers/class-default-content-installer.php (1)
inc/class-requirements.php (1)
is_multisite(228-237)
🔇 Additional comments (3)
inc/admin-pages/class-base-admin-page.php (1)
219-224: Docblock fix looks goodAccurate and clarifies the method purpose.
inc/installers/class-default-content-installer.php (1)
55-57: Good guard to avoid fatal in single-siteEarly return prevents calling domain_exists() outside multisite. Nice.
inc/admin-pages/class-setup-wizard-admin-page.php (1)
224-236: Redirect split is correct and avoids loopsClear separation of single-site vs network flows, with exits. LGTM.
# Conflicts: # inc/class-wp-ultimo.php
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
- Guard multisite-only functions (switch_to_blog, get_blog_details) with is_multisite() checks to prevent fatal errors on single-site installs - Defer Multisite_Setup_Admin_Page instantiation to init action and only load on non-multisite installations - Add admin_init redirect to guide users to the appropriate setup wizard - Fix wp-config.php constant detection to use regex instead of strpos, preventing false matches on commented-out constants - Fix First Steps dashboard widget not recognizing setup completion when stored as a timestamp (wu_string_to_bool fails on timestamp strings) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inc/admin-pages/class-setup-wizard-admin-page.php (1)
228-240:⚠️ Potential issue | 🔴 CriticalOverly aggressive redirect — blocks all admin pages for all users.
The non-multisite branch (line 229) redirects every
admin_initrequest to the multisite setup page whenever multisite is not enabled. This means:
- All admin users (editors, authors, etc.) — not just the network admin — are trapped in an infinite redirect loop to a page they likely can't act on. Add a
current_user_can('manage_options')guard.- All admin pages are blocked (posts, media, settings, etc.), not just plugin pages. Consider restricting this to plugin-related pages, or at minimum gating it behind a transient/option that's only set on activation.
- AJAX requests (
admin_initfires foradmin-ajax.phptoo) will also be redirected, potentially breaking other functionality.Suggested minimal fix
public function redirect_to_wizard(): void { + // Only redirect for users who can manage options + if ( ! current_user_can('manage_options')) { + return; + } + + // Don't redirect AJAX requests + if ( wp_doing_ajax() ) { + return; + } + // If multisite is not enabled, redirect to multisite setup page if ( ! is_multisite() && wu_request('page') !== 'wp-ultimo-multisite-setup') { wp_safe_redirect(admin_url('admin.php?page=wp-ultimo-multisite-setup')); exit; }inc/functions/site.php (1)
16-22:⚠️ Potential issue | 🟠 MajorNull return introduces widespread NPE risk across codebase.
The docblock declares
@return \WP_Ultimo\Models\Sitebut the function returnsnullfor non-multisite installations (lines 20-21). This creates a mismatch that misleads developers into unsafe code.At least 25+ callers chain method calls on
wu_get_current_site()without null protection, including:
inc/class-dashboard-widgets.php:143—wu_get_current_site()->get_type()inc/ui/class-base-element.php:649—wu_get_current_site()->get_type()inc/limits/class-theme-limits.php— Multiple calls tohas_limitations(),get_limitations(), etc.inc/limits/class-post-type-limits.php— Multiple calls tohas_limitations(),get_limitations()inc/limits/class-plugin-limits.php,class-disk-space-limits.php,class-customer-user-role-limits.php— Similar patternsAll of these will fatal error if the function returns null.
Required fixes:
- Update the
@returntype to\WP_Ultimo\Models\Site|null.- Decide on a systematic solution: either add null guards to all 25+ callers, or reconsider whether the function should return null at all vs. raising an error or returning a default object.
🤖 Fix all issues with AI agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php`:
- Around line 283-285: The call to wp_die currently uses __() which is flagged
by static analysis; replace the unescaped translation with an escaped
translation function by changing the wp_die call to use esc_html__() instead of
__(), i.e. update the wp_die invocation that follows the
current_user_can('manage_options') check so the message is escaped (refer to the
wp_die(...) and current_user_can(...) usage in
class-multisite-setup-admin-page.php).
- Around line 525-526: Replace the use of PHP's parse_url() with WordPress'
wp_parse_url() for both assignments to $base and $domain to avoid PHP-version
inconsistencies: call wp_parse_url(trailingslashit(get_option('home')),
PHP_URL_PATH) for $base and wp_parse_url(get_option('home'), PHP_URL_HOST) for
$domain (keeping trailingslashit/get_option usage and the PHP_URL_* component
constants) so the code uses WP's URL parser consistently.
- Line 219: Replace uses of PHP's parse_url() with WordPress's wp_parse_url() to
avoid inconsistent outputs across PHP versions: update the assignment that sets
$base_domain (currently using parse_url($home_url, PHP_URL_HOST)) to use
wp_parse_url($home_url)['host'] (or check the returned array safely) and make
the corresponding change inside the display_manual_instructions() function where
parse_url() is used; ensure you handle the wp_parse_url() return structure and
fallback if 'host' is not set.
- Around line 465-500: modify_wp_config() currently uses non‑Yoda conditions and
direct PHP filesystem functions and ignores wp-config.php located one directory
above ABSPATH; update it to use WP_Filesystem for existence/read/write
operations (replace file_exists/is_writable/file_get_contents/file_put_contents
with appropriate WP_Filesystem methods after initializing global
$wp_filesystem), change comparisons to Yoda style (e.g., false === $var checks
for file_get_contents/file_put_contents results and === false for strpos), and
add a fallback check for wp-config.php in the parent directory (try ABSPATH .
'wp-config.php' then dirname(ABSPATH) . '/wp-config.php') before proceeding;
keep the existing logic that checks preg_match for WP_ALLOW_MULTISITE and uses
substr_replace to insert $constant_to_add but perform reads/writes via
$wp_filesystem methods instead of direct PHP calls.
- Around line 579-589: In add_final_multisite_constants(): replace the fragile
exact-string anchor search for "define( 'WP_ALLOW_MULTISITE', true );" with a
regex that matches a define of WP_ALLOW_MULTISITE allowing optional whitespace
and variations (e.g. /define\s*\(\s*'WP_ALLOW_MULTISITE'\s*,\s*true\s*\)\s*;/i),
set $insert_position to the end of that regex match if found; if not found,
fallback to locating the "/* That's all" marker (e.g. strpos($config_content,
"/* That's all") ) and insert the $constants_to_add before that; finally, use a
Yoda-style comparison when writing the file (false !== file_put_contents(...))
and return false only if neither anchor is found or the write fails.
| $wp_config_path = ABSPATH . 'wp-config.php'; | ||
|
|
||
| if (! file_exists($wp_config_path) || ! is_writable($wp_config_path)) { | ||
| return false; | ||
| } | ||
|
|
||
| $config_content = file_get_contents($wp_config_path); | ||
|
|
||
| if ($config_content === false) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if WP_ALLOW_MULTISITE is already actively defined (not commented out) | ||
| if (preg_match('/^\s*define\s*\(\s*[\'"]WP_ALLOW_MULTISITE[\'"]/m', $config_content)) { | ||
| return true; // Already configured | ||
| } | ||
|
|
||
| // Find the location to insert the constant | ||
| $search = "/* That's all, stop editing! Happy publishing. */"; | ||
| $insert_position = strpos($config_content, $search); | ||
|
|
||
| if ($insert_position === false) { | ||
| // Fallback: look for the wp-settings.php include | ||
| $search = "require_once ABSPATH . 'wp-settings.php';"; | ||
| $insert_position = strpos($config_content, $search); | ||
| } | ||
|
|
||
| if ($insert_position === false) { | ||
| return false; // Can't find a safe place to insert | ||
| } | ||
|
|
||
| $constant_to_add = "\n// Multisite Ultimate: Enable WordPress Multisite\ndefine( 'WP_ALLOW_MULTISITE', true );\n\n"; | ||
|
|
||
| $new_content = substr_replace($config_content, $constant_to_add, $insert_position, 0); | ||
|
|
||
| return file_put_contents($wp_config_path, $new_content) !== false; |
There was a problem hiding this comment.
Multiple static analysis findings in modify_wp_config().
- Yoda conditions (lines 473, 486, 492): The project coding standard requires Yoda-style checks.
is_writable()(line 467): Coding standards preferWP_Filesystemmethods over direct PHP filesystem calls.wp-config.phpone directory above ABSPATH is a common WordPress setup and isn't supported here (noted in a prior review).
Yoda condition fixes
- if ($config_content === false) {
+ if (false === $config_content) {
- if ($insert_position === false) {
+ if (false === $insert_position) {
// Fallback: look for the wp-settings.php include
- if ($insert_position === false) {
+ if (false === $insert_position) {
return false; // Can't find a safe place to insert🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 492-492:
Use Yoda Condition checks, you must.
[failure] 486-486:
Use Yoda Condition checks, you must.
[failure] 473-473:
Use Yoda Condition checks, you must.
[warning] 467-467:
File operations should use WP_Filesystem methods instead of direct PHP filesystem calls. Found: is_writable().
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php` around lines 465 - 500,
modify_wp_config() currently uses non‑Yoda conditions and direct PHP filesystem
functions and ignores wp-config.php located one directory above ABSPATH; update
it to use WP_Filesystem for existence/read/write operations (replace
file_exists/is_writable/file_get_contents/file_put_contents with appropriate
WP_Filesystem methods after initializing global $wp_filesystem), change
comparisons to Yoda style (e.g., false === $var checks for
file_get_contents/file_put_contents results and === false for strpos), and add a
fallback check for wp-config.php in the parent directory (try ABSPATH .
'wp-config.php' then dirname(ABSPATH) . '/wp-config.php') before proceeding;
keep the existing logic that checks preg_match for WP_ALLOW_MULTISITE and uses
substr_replace to insert $constant_to_add but perform reads/writes via
$wp_filesystem methods instead of direct PHP calls.
- Fix radio boolean conversion: use '0'/'1' keys instead of 'sub0'/'sub1' and compare with === '1' instead of (bool) cast (#3) - Fix namespace: catch (\Exception) instead of catch (Exception) (#4) - Use regex for flexible WP_ALLOW_MULTISITE anchor matching with fallback insertion point in add_final_multisite_constants() (#5) - Propagate add_final_multisite_constants() failure to create_network() (#6) - Support wp-config.php one level above ABSPATH via get_wp_config_path() (#7) - Replace parse_url() with wp_parse_url() at all 4 locations (#8) - Escape wp_die() output with esc_html__() (#9) - Use Yoda conditions for false comparisons (#10) - Wire up welcome view callback in sections array (#12) - Fix inline script handle from 'wp-admin' to 'jquery' (#13) - Provide subdomain-specific .htaccess rules in manual instructions (#15) - Guard redirect_to_wizard() with wp_doing_ajax() and capability check (#1) - Update wu_get_current_site() @return type to include null (#2) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/admin-pages/class-setup-wizard-admin-page.php (1)
228-244:⚠️ Potential issue | 🔴 CriticalCapability mismatch locks out regular site admins on multisite.
Line 228 checks only
manage_options, but the multisite branch (line 240) redirects towu_network_admin_url('wp-ultimo-setup'), which is a network admin page requiringmanage_network. On multisite, a regular site admin hasmanage_optionsbut notmanage_network— they'll pass the capability check at line 228, trigger the redirect at line 240, and be sent to a network admin page they cannot access, effectively locking them out of the dashboard on every page load.The capability checks should be branch-specific:
manage_optionsfor the non-multisite branch andmanage_networkfor the multisite branch.Proposed fix
- if (wp_doing_ajax() || ! current_user_can('manage_options')) { + if (wp_doing_ajax()) { return; } // If multisite is not enabled, redirect to multisite setup page - if ( ! is_multisite() && wu_request('page') !== 'wp-ultimo-multisite-setup') { + if ( ! is_multisite() && current_user_can('manage_options') && wu_request('page') !== 'wp-ultimo-multisite-setup') { wp_safe_redirect(admin_url('admin.php?page=wp-ultimo-multisite-setup')); exit; } // If multisite is enabled but setup is not finished, redirect to setup wizard - if ( is_multisite() && ! Requirements::run_setup() && wu_request('page') !== 'wp-ultimo-setup') { + if ( is_multisite() && current_user_can('manage_network') && ! Requirements::run_setup() && wu_request('page') !== 'wp-ultimo-setup') { wp_safe_redirect(wu_network_admin_url('wp-ultimo-setup')); exit; }
🤖 Fix all issues with AI agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php`:
- Around line 646-673: The inline clipboard script added in register_scripts()
targets buttons matching selector
button[onclick*='navigator.clipboard.writeText'] but the manual instructions
render only <pre> blocks (no such buttons), so either remove the dead
wp_add_inline_script call in register_scripts() or add actual copy buttons next
to the rendered <pre> blocks and bind them to navigator.clipboard.writeText;
locate register_scripts(), the wp_add_inline_script(...) block and the query
selector string "button[onclick*='navigator.clipboard.writeText']" to decide
whether to delete the inline script or implement UI buttons that match that
selector and update the click handler accordingly.
- Around line 257-270: The backup_warning 'desc' currently concatenates raw
__('...','multisite-ultimate') outputs into HTML; replace those calls with
escaped versions (esc_html__() or esc_html_e() where appropriate) for the text
nodes inside the HTML string to prevent unescaped translated markup, e.g. change
the __('Before You Continue', 'multisite-ultimate') and the long paragraph
__('Please ensure...', 'multisite-ultimate') usages in the backup_warning array
so they are escaped before concatenation; keep icon/HTML markup intact but
ensure only escaped translation functions are injected into the 'desc' value.
- Around line 229-238: The translatable strings inside the 'subdomain_install'
settings array use placeholders but lack translator context comments; add a /*
translators: ... */ comment immediately above each
__()/esc_html__()/esc_html__() call that contains placeholders to explain what
the placeholders represent (e.g., base domain and example site), so translators
know how to order/translate them; do the same for the other occurrence that uses
esc_html__ with %1$s / %2$s (referenced in the review) so both spots include
clear translators: notes.
🧹 Nitpick comments (2)
inc/admin-pages/class-multisite-setup-admin-page.php (2)
610-617: Consider sanitizing$domainbefore interpolating into a PHPdefine()statement.Line 614 interpolates
$domaindirectly into a string that gets written towp-config.php. Whilewp_parse_url(..., PHP_URL_HOST)should return a safe hostname, a defensiveaddcslashes($domain, "'\\")or strict validation (e.g., regex for valid hostname characters) would prevent any edge-case corruption of the config file.♻️ Proposed defensive fix
+ // Ensure domain is safe for inclusion in a PHP string literal + $safe_domain = preg_replace('/[^a-zA-Z0-9.\-:]/', '', $domain); + $constants_to_add = "\n// Multisite Ultimate: Multisite Configuration\n"; $constants_to_add .= "define( 'MULTISITE', true );\n"; $constants_to_add .= "define( 'SUBDOMAIN_INSTALL', " . ($subdomain_install ? 'true' : 'false') . " );\n"; - $constants_to_add .= "define( 'DOMAIN_CURRENT_SITE', '{$domain}' );\n"; + $constants_to_add .= "define( 'DOMAIN_CURRENT_SITE', '{$safe_domain}' );\n"; $constants_to_add .= "define( 'PATH_CURRENT_SITE', '/' );\n";
319-359: Transient-based state between wizard steps is fragile.
section_completereads transients set byhandle_configure(Lines 291–306). Transients can be evicted early by a persistent object cache, or expire if the user is slow. If transients are missing, both$wp_config_modifiedand$network_createdwill befalse, which safely falls back to manual instructions — so the failure mode is acceptable, but the user experience would be confusing (they'd see "manual instructions" even though setup succeeded).Consider using a short-lived user meta or site option as a more reliable transport mechanism, or at minimum check the actual multisite state (
is_multisite()) as a ground-truth signal insection_complete.
| 'subdomain_install' => [ | ||
| 'type' => 'radio', | ||
| 'title' => __('Site Structure', 'multisite-ultimate'), | ||
| 'desc' => __('Choose between subdirectories or subdomains for your network sites.', 'multisite-ultimate'), | ||
| 'options' => [ | ||
| '0' => sprintf(__('Sites will use sub-directories like %s (Recommended)', 'multisite-ultimate'), '<code>' . esc_html($base_domain) . '/site1</code>'), | ||
| '1' => sprintf(__('Sites will use sub-domains like %s (Requires wildcard DNS)', 'multisite-ultimate'), '<code>site1.' . esc_html($base_domain) . '</code>'), | ||
| ], | ||
| 'default' => '0', | ||
| ], |
There was a problem hiding this comment.
Missing translators: comments for translatable strings with placeholders.
Static analysis requires a translators: comment above __() / esc_html__() calls that contain placeholders, to help translators understand context.
Proposed fix
'options' => [
+ // translators: %s is an example URL showing the subdirectory structure, e.g. example.com/site1
'0' => sprintf(__('Sites will use sub-directories like %s (Recommended)', 'multisite-ultimate'), '<code>' . esc_html($base_domain) . '/site1</code>'),
- '1' => sprintf(__('Sites will use sub-domains like %s (Requires wildcard DNS)', 'multisite-ultimate'), '<code>site1.' . esc_html($base_domain) . '</code>'),
+ // translators: %s is an example URL showing the subdomain structure, e.g. site1.example.com
+ '1' => sprintf(__('Sites will use sub-domains like %s (Requires wildcard DNS)', 'multisite-ultimate'), '<code>site1.' . esc_html($base_domain) . '</code>'),The same issue applies at Line 427 (esc_html__ with %1$s / %2$s placeholders).
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 235-235:
A function call to __() with texts containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.
[failure] 234-234:
A function call to __() with texts containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php` around lines 229 - 238,
The translatable strings inside the 'subdomain_install' settings array use
placeholders but lack translator context comments; add a /* translators: ... */
comment immediately above each __()/esc_html__()/esc_html__() call that contains
placeholders to explain what the placeholders represent (e.g., base domain and
example site), so translators know how to order/translate them; do the same for
the other occurrence that uses esc_html__ with %1$s / %2$s (referenced in the
review) so both spots include clear translators: notes.
| 'backup_warning' => [ | ||
| 'type' => 'note', | ||
| 'desc' => '<div class="wu-bg-yellow-50 wu-border wu-border-yellow-200 wu-rounded-lg wu-p-4"> | ||
| <div class="wu-flex"> | ||
| <div class="wu-flex-shrink-0"> | ||
| <span class="dashicons dashicons-warning wu-text-yellow-500"></span> | ||
| </div> | ||
| <div class="wu-ml-3"> | ||
| <h4 class="wu-text-sm wu-font-medium wu-text-yellow-800">' . __('Before You Continue', 'multisite-ultimate') . '</h4> | ||
| <p class="wu-text-sm wu-text-yellow-700 wu-mt-1">' . __('Please ensure you have a recent backup of your website files and database. The multisite setup process will modify your wp-config.php file and create new database tables.', 'multisite-ultimate') . '</p> | ||
| </div> | ||
| </div> | ||
| </div>', | ||
| ], |
There was a problem hiding this comment.
Unescaped __() output concatenated into HTML.
The 'desc' value for backup_warning embeds __() calls directly in raw HTML. If a translation contains malicious markup, it would be rendered unescaped. Use esc_html__() for text nodes within the HTML.
Proposed fix
- <h4 class="wu-text-sm wu-font-medium wu-text-yellow-800">' . __('Before You Continue', 'multisite-ultimate') . '</h4>
- <p class="wu-text-sm wu-text-yellow-700 wu-mt-1">' . __('Please ensure you have a recent backup of your website files and database. The multisite setup process will modify your wp-config.php file and create new database tables.', 'multisite-ultimate') . '</p>
+ <h4 class="wu-text-sm wu-font-medium wu-text-yellow-800">' . esc_html__('Before You Continue', 'multisite-ultimate') . '</h4>
+ <p class="wu-text-sm wu-text-yellow-700 wu-mt-1">' . esc_html__('Please ensure you have a recent backup of your website files and database. The multisite setup process will modify your wp-config.php file and create new database tables.', 'multisite-ultimate') . '</p>🤖 Prompt for AI Agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php` around lines 257 - 270,
The backup_warning 'desc' currently concatenates raw
__('...','multisite-ultimate') outputs into HTML; replace those calls with
escaped versions (esc_html__() or esc_html_e() where appropriate) for the text
nodes inside the HTML string to prevent unescaped translated markup, e.g. change
the __('Before You Continue', 'multisite-ultimate') and the long paragraph
__('Please ensure...', 'multisite-ultimate') usages in the backup_warning array
so they are escaped before concatenation; keep icon/HTML markup intact but
ensure only escaped translation functions are injected into the 'desc' value.
| public function register_scripts(): void { | ||
|
|
||
| if (get_current_screen()->id !== 'toplevel_page_wp-ultimo-multisite-setup') { | ||
| return; | ||
| } | ||
|
|
||
| wp_add_inline_script( | ||
| 'jquery', | ||
| ' | ||
| // Copy to clipboard functionality | ||
| document.addEventListener("DOMContentLoaded", function() { | ||
| document.querySelectorAll("button[onclick*=\'navigator.clipboard.writeText\']").forEach(function(button) { | ||
| button.addEventListener("click", function() { | ||
| var textarea = this.nextElementSibling; | ||
| if (textarea && textarea.tagName === "TEXTAREA") { | ||
| navigator.clipboard.writeText(textarea.value).then(function() { | ||
| button.textContent = "Copied!"; | ||
| setTimeout(function() { | ||
| button.textContent = "Copy to clipboard"; | ||
| }, 2000); | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| ' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Clipboard JS targets buttons that don't exist in the rendered output.
The inline script queries button[onclick*='navigator.clipboard.writeText'] but the manual instructions section (Lines 413–471) renders <pre> blocks without any copy buttons. This script appears to be dead code on this page. Either add copy-to-clipboard buttons to the manual instructions or remove this script.
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-multisite-setup-admin-page.php` around lines 646 - 673,
The inline clipboard script added in register_scripts() targets buttons matching
selector button[onclick*='navigator.clipboard.writeText'] but the manual
instructions render only <pre> blocks (no such buttons), so either remove the
dead wp_add_inline_script call in register_scripts() or add actual copy buttons
next to the rendered <pre> blocks and bind them to
navigator.clipboard.writeText; locate register_scripts(), the
wp_add_inline_script(...) block and the query selector string
"button[onclick*='navigator.clipboard.writeText']" to decide whether to delete
the inline script or implement UI buttons that match that selector and update
the click handler accordingly.
Checkout rewrite rules (e.g. /register/plan-slug) depend on the registration page's slug, but flush_rewrite_rules() was only called on settings page save or plugin update. This left stale/missing rules when pages were created by the setup wizard or had their slug changed. - Hook save_post_page and wp_trash_post in Checkout_Pages to flush rewrite rules when any signup page is modified - Flush rewrite rules in Default_Content_Installer after creating the registration page during setup - Flush rewrite rules in Migrator after creating the registration page during v1→v2 migration - Fix docblock @param and @return types in Current class Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
- Change network type from radio buttons to select dropdown - Default to subdomains (recommended) instead of subdirectories - Add translators comments for placeholder strings - Use WP_Filesystem instead of direct is_writable() calls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
…step - Fix Base_Installer::handle() to return WP_Error on failure instead of silently swallowing exceptions - Fix setup_install() to check return value and send wp_send_json_error on failure - Register missing wp_ajax_wu_multisite_install AJAX action - Fix JS error callback to display error messages instead of blank text - Add network_activate step to Multisite_Network_Installer - Update Recommended_Plugins_Installer and Migrator handle() to return status for filter chain - Change wu_handle_ajax_installers from add_action to add_filter (matches apply_filters usage) - Configure PHPCS to not exit non-zero on warnings (only errors) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation